Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): make import/no-extraneous-dependencies work for react config #21924

Merged

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Mar 2, 2022

Current Behavior

  • packages being published don't specify proper dependencies
  • lint rule checking package.json dependency compliance won't error if there are violations
  • we ship packages to npm that may not be usable after install

image

New Behavior

  • all packages specify proper dependencies
  • lint rule will error out if there are violations

image
image

additional feature implemented

  • just-scripts copy task, adds new api to copyTo ( copy and rename )
{
  "copyTo": {
    // greet.jpg file will be copied to hello-world directory
    "hello-world": ["./assets/greet.jpg"],

    // readme.md__tmpl__ file will be copied production directory and renamed to readme.md
   "production/readme.md": "src/one/two/readme.md__tmpl__"
  }
}

Related Issue(s)

Fixes #21789

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 2, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0bcc3bc:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Mar 2, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: e6fa9864d66e5cdf4e965b6069f28c4455dd35dc (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 2, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-accordion
Accordion (including children components)
73.035 kB
22.11 kB
react-avatar
Avatar
44.982 kB
13.059 kB
react-badge
Badge
20.869 kB
6.549 kB
react-badge
CounterBadge
21.737 kB
6.843 kB
react-badge
PresenceBadge
21.866 kB
6.536 kB
react-button
Button
28.013 kB
8.059 kB
react-button
CompoundButton
33.336 kB
9.037 kB
react-button
MenuButton
29.763 kB
8.656 kB
react-button
SplitButton
36.235 kB
9.857 kB
react-button
ToggleButton
37.395 kB
8.68 kB
react-card
Card - All
53.205 kB
15.27 kB
react-card
Card
48.898 kB
14.083 kB
react-card
CardFooter
7.653 kB
3.246 kB
react-card
CardHeader
8.931 kB
3.689 kB
react-card
CardPreview
7.626 kB
3.272 kB
react-combobox
Combobox
6.813 kB
2.895 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
172.513 kB
48.34 kB
react-components
react-components: FluentProvider & webLightTheme
32.526 kB
10.645 kB
react-divider
Divider
15.301 kB
5.532 kB
react-image
Image
10.105 kB
3.952 kB
react-input
Input
21.538 kB
7.134 kB
react-label
Label
8.341 kB
3.487 kB
react-link
Link
11.102 kB
4.504 kB
react-menu
Menu (including children components)
103.976 kB
31.998 kB
react-menu
Menu (including selectable components)
106.331 kB
32.361 kB
react-popover
Popover
96.776 kB
29.553 kB
react-portal
Portal
6.267 kB
2.168 kB
react-positioning
usePopper
23.21 kB
8.084 kB
react-provider
FluentProvider
14.009 kB
5.25 kB
react-select
Select
7.754 kB
3.258 kB
react-slider
Slider
23.351 kB
7.894 kB
react-spinner
Spinner
6.811 kB
2.895 kB
react-switch
Switch
22.598 kB
7.642 kB
react-text
Text - Default
10.793 kB
4.23 kB
react-text
Text - Wrappers
14.107 kB
4.573 kB
react-theme
Single theme token import
69 B
89 B
react-theme
Teams: all themes
29.426 kB
6.551 kB
react-theme
Teams: Light theme
18.42 kB
5.27 kB
react-tooltip
Tooltip
42.76 kB
14.701 kB
react-utilities
SSRProvider
189 B
161 B
🤖 This report was generated against e6fa9864d66e5cdf4e965b6069f28c4455dd35dc

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 2, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AvatarMinimalPerf.default 178 167 1.07:1
RadioGroupMinimalPerf.default 394 374 1.05:1
SegmentMinimalPerf.default 300 287 1.05:1
ChatMinimalPerf.default 638 616 1.04:1
ListMinimalPerf.default 443 425 1.04:1
TableMinimalPerf.default 352 339 1.04:1
FlexMinimalPerf.default 251 243 1.03:1
GridMinimalPerf.default 296 286 1.03:1
HeaderMinimalPerf.default 312 303 1.03:1
LayoutMinimalPerf.default 314 306 1.03:1
TreeWith60ListItems.default 155 150 1.03:1
FormMinimalPerf.default 352 344 1.02:1
HeaderSlotsPerf.default 646 631 1.02:1
LabelMinimalPerf.default 327 321 1.02:1
RosterPerf.default 984 960 1.02:1
ReactionMinimalPerf.default 319 313 1.02:1
RefMinimalPerf.default 207 202 1.02:1
StatusMinimalPerf.default 574 562 1.02:1
TableManyItemsPerf.default 1620 1596 1.02:1
ToolbarMinimalPerf.default 816 797 1.02:1
TreeMinimalPerf.default 689 675 1.02:1
AnimationMinimalPerf.default 468 463 1.01:1
ButtonOverridesMissPerf.default 1440 1426 1.01:1
ButtonSlotsPerf.default 470 467 1.01:1
CheckboxMinimalPerf.default 2259 2242 1.01:1
DividerMinimalPerf.default 304 301 1.01:1
InputMinimalPerf.default 1102 1091 1.01:1
LoaderMinimalPerf.default 581 577 1.01:1
SkeletonMinimalPerf.default 300 296 1.01:1
SplitButtonMinimalPerf.default 3693 3654 1.01:1
TextMinimalPerf.default 295 293 1.01:1
AlertMinimalPerf.default 234 233 1:1
CarouselMinimalPerf.default 409 407 1:1
DropdownManyItemsPerf.default 566 565 1:1
DropdownMinimalPerf.default 2585 2575 1:1
EmbedMinimalPerf.default 3479 3492 1:1
ItemLayoutMinimalPerf.default 993 993 1:1
ListNestedPerf.default 467 466 1:1
MenuMinimalPerf.default 713 716 1:1
MenuButtonMinimalPerf.default 1409 1402 1:1
PortalMinimalPerf.default 156 156 1:1
ProviderMergeThemesPerf.default 1458 1460 1:1
ProviderMinimalPerf.default 972 969 1:1
SliderMinimalPerf.default 1427 1420 1:1
TextAreaMinimalPerf.default 403 405 1:1
TooltipMinimalPerf.default 865 866 1:1
VideoMinimalPerf.default 550 548 1:1
AttachmentMinimalPerf.default 137 138 0.99:1
DatepickerMinimalPerf.default 4616 4655 0.99:1
DialogMinimalPerf.default 639 644 0.99:1
PopupMinimalPerf.default 528 531 0.99:1
CustomToolbarPrototype.default 3428 3458 0.99:1
BoxMinimalPerf.default 287 294 0.98:1
CardMinimalPerf.default 454 463 0.98:1
ChatDuplicateMessagesPerf.default 252 258 0.98:1
ListWith60ListItems.default 549 561 0.98:1
IconMinimalPerf.default 507 518 0.98:1
AttachmentSlotsPerf.default 903 935 0.97:1
ChatWithPopoverPerf.default 332 343 0.97:1
ImageMinimalPerf.default 303 311 0.97:1
AccordionMinimalPerf.default 133 138 0.96:1
ButtonMinimalPerf.default 139 146 0.95:1
ListCommonPerf.default 519 554 0.94:1

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 2, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 940 952 5000
Breadcrumb mount 2717 2687 1000
Checkbox mount 1553 1545 5000
CheckboxBase mount 1283 1267 5000
ChoiceGroup mount 4846 4763 5000
ComboBox mount 1008 1000 1000
CommandBar mount 10450 10537 1000
ContextualMenu mount 11719 11760 1000
DefaultButton mount 1141 1158 5000
DetailsRow mount 3904 3926 5000
DetailsRowFast mount 3894 3879 5000
DetailsRowNoStyles mount 3724 3722 5000
Dialog mount 2231 2299 1000
DocumentCardTitle mount 165 169 1000
Dropdown mount 3293 3332 5000
FocusTrapZone mount 1813 1833 5000
FocusZone mount 1867 1777 5000
IconButton mount 1777 1790 5000
Label mount 357 354 5000
Layer mount 2962 3064 5000
Link mount 479 473 5000
MenuButton mount 1504 1487 5000
MessageBar mount 2176 2126 5000
Nav mount 3400 3390 1000
OverflowSet mount 1107 1105 5000
Panel mount 2196 2170 1000
Persona mount 855 857 1000
Pivot mount 1506 1485 1000
PrimaryButton mount 1345 1302 5000
Rating mount 7883 7933 5000
SearchBox mount 1338 1345 5000
Shimmer mount 2509 2501 5000
Slider mount 1995 1935 5000
SpinButton mount 5121 5105 5000
Spinner mount 428 449 5000
SplitButton mount 3282 3262 5000
Stack mount 548 531 5000
StackWithIntrinsicChildren mount 2357 2358 5000
StackWithTextChildren mount 5321 5368 5000
SwatchColorPicker mount 11910 11911 5000
TagPicker mount 2786 2752 5000
TeachingBubble mount 93033 93435 5000
Text mount 432 430 5000
TextField mount 1423 1436 5000
ThemeProvider mount 1188 1187 5000
ThemeProvider virtual-rerender 654 639 5000
ThemeProvider virtual-rerender-with-unmount 1998 1939 5000
Toggle mount 807 804 5000
buttonNative mount 127 119 5000

@Hotell Hotell force-pushed the hotell/21789-lint-rules-import-resolver branch from 838ea34 to 3868fba Compare March 3, 2022 13:28
'@typescript-eslint/parser': ['.ts', '.tsx'],
},
'import/resolver': {
typescript: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this configuration was breaking import/no-extraneous-dependencies to work. now that typescript plugin is not being used at all and recommended configuration provided by import plugin is being used instead.

/**
*
* import plugin rules
* @see https://github.com/import-js/eslint-plugin-import
Copy link
Contributor Author

@Hotell Hotell Mar 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collocated all import plugins rules to one place so its easier to navigate/read

@Hotell Hotell force-pushed the hotell/21789-lint-rules-import-resolver branch 2 times, most recently from 276c9f1 to 2f63a34 Compare March 7, 2022 11:51
@@ -12,7 +12,6 @@
"license": "MIT",
"devDependencies": {
"@fluentui/eslint-plugin": "*",
"@fluentui/scripts": "^1.0.0",
"@microsoft/eslint-plugin-sdl": "0.1.9"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applied single version policy for devDeps

},
"dependencies": {
"@fluentui/react-conformance": "^0.10.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: create issue to get rid of v8 deps in v9 tree

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -409,6 +408,14 @@
"ci-info",
"node-fetch"
]
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version group needed, because we cannot specify * as proper dependency version

@@ -277,24 +250,40 @@ const config = {
'react/no-unused-prop-types': 'off',
'react/prefer-es6-class': 'off',

'jsdoc/check-tag-names': [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no changes here - this occurs as diff because moving of lines....

@@ -1,5 +1,5 @@
{
"copyTo": {
"unstable": ["./src/unstable/package.json"]
"unstable/package.json": "./src/unstable/package.json__tmpl__"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leveraging new API : copy and rename to destination key

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

package.json is using tmpl suffix used by nx, but there are no variables being interpolated within the file. the main purpose of this is so import/no-extreanous won't check this file ( which would trigger lint errors that are invalid )

@@ -1,4 +0,0 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is not needed, unstable is already in check path of tsconfig.lib.json

},
"dependencies": {
"@fluentui/react-conformance": "^0.10.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -166,7 +166,6 @@ const useContentStyles = makeStyles({
/**
* Apply styling to the Tab slots based on the state
*/
// eslint-disable-next-line @typescript-eslint/naming-convention
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 removed unused lint ignores


export function expandSourcePath(pattern) {
export function expandSourcePath(pattern: string): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

improved types

);
const destinationPath = path.resolve(process.cwd(), destination);

if (Array.isArray(sources)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now there is new api:

  • the old one works as before ( value is a array of string paths/globs)
  • the new one is destination is the path including new file name + value is only string path (copy + rename)

@Hotell Hotell marked this pull request as ready for review March 7, 2022 14:45
@Hotell Hotell requested review from a team, GeoffCoxMSFT, behowell and khmakoto as code owners March 7, 2022 14:45
@Hotell Hotell force-pushed the hotell/21789-lint-rules-import-resolver branch from 0cb8044 to 2fb4bd5 Compare March 7, 2022 14:49
@Hotell Hotell marked this pull request as draft March 7, 2022 18:29
@Hotell
Copy link
Contributor Author

Hotell commented Mar 7, 2022

had great brainstorming with @layershifter today and we wanna add back 'import/resolver#typescript' for consistency ( althought import plugin will work in our use case as every package is symlinked to root node_modules because yarn workspaces ), thus moving back to draft, please don't review yet

@andrefcdias andrefcdias changed the title fix(eslint-plugin): make import/no-extraneous-dependencies work for r… fix(eslint-plugin): make import/no-extraneous-dependencies work for react config Mar 8, 2022
@Hotell Hotell force-pushed the hotell/21789-lint-rules-import-resolver branch 2 times, most recently from 088ed06 to 570c756 Compare March 8, 2022 15:53
@Hotell Hotell marked this pull request as ready for review March 8, 2022 17:47
@Hotell Hotell requested a review from layershifter March 8, 2022 17:48
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, thanks for fixing this ❤️

@andrefcdias andrefcdias self-requested a review March 9, 2022 13:18
@spmonahan
Copy link
Contributor

Look at your example screenshots, should we have a Prettier/eslint rule that merges the two from "@fluentui/react-utilitites" lines into a single line?

@Hotell
Copy link
Contributor Author

Hotell commented Mar 10, 2022

Look at your example screenshots, should we have a Prettier/eslint rule that merges the two from "@fluentui/react-utilitites" lines into a single line?

absolutely yes. it is planned (this/next month) to cover these kind of things (also this one #18594)

created #22049

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 10, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1286 1283 5000
Button mount 763 760 5000
FluentProvider mount 2268 2141 5000
FluentProviderWithTheme mount 394 400 10
FluentProviderWithTheme virtual-rerender 349 338 10
FluentProviderWithTheme virtual-rerender-with-unmount 428 453 10
MakeStyles mount 1999 1916 50000

@Hotell Hotell merged commit 8722596 into microsoft:master Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

import/no-extraneous-dependencies does not work as expected
10 participants